Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

set container of kubectl to 'uncaptured' #1664

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

set container of kubectl to 'uncaptured' #1664

wants to merge 1 commit into from

Conversation

silenceshell
Copy link
Member

When trying the perf/benchmark, I met a problem below.
The pods fortioclient-54894c5758-htdgl default container is captured, so the kubectl command will try to run fortio in captured container.
Here is the log.

(benchmark) root@bbf4b2393670:~/tools-master/perf/benchmark# python runner/runner.py --conn 10  --qps 100,500,1000,2000,4000 --duration 240 --load_gen_type=fortio --telemetry_mode=v2-nullvm --perf=true

-------------- Running in both mode --------------
ed78b58c_qps_100_c_10_1024_v2-nullvmbothsidecars_perf.data
kubectl --namespace twopods-istio exec fortioclient-54894c5758-htdgl  -- fortio load  -jitter=False -c 10 -qps 100 -t 240s -a -r 0.001   -httpbufferkb=128 -labels ed78b58c_qps_100_c_10_1024_v2-nullvm_both http://fortioserver:8080/echo?size=1024
Defaulting container name to captured.
Use 'kubectl describe pod/fortioclient-54894c5758-htdgl -n twopods-istio' to see all of the containers in this pod.
OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "exec: \"fortio\": executable file not found in $PATH": unknown
command terminated with exit code 126

Maybe it is better to specify the container to run by kubectl, thus we won't exec fortio in wrong container.

@silenceshell silenceshell requested a review from a team as a code owner August 3, 2021 11:28
@istio-policy-bot
Copy link

😊 Welcome @silenceshell! This is either your first contribution to the Istio tools repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 3, 2021
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Aug 3, 2021
@silenceshell silenceshell marked this pull request as draft August 3, 2021 16:00
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Aug 3, 2021
@silenceshell silenceshell force-pushed the set-default-container-to-uncaptured branch from 303e710 to fdfcaea Compare August 4, 2021 02:25
@silenceshell silenceshell marked this pull request as ready for review August 4, 2021 02:48
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Aug 4, 2021
@silenceshell silenceshell marked this pull request as draft August 4, 2021 10:06
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Aug 4, 2021
@silenceshell silenceshell marked this pull request as ready for review August 4, 2021 13:44
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Aug 4, 2021
@silenceshell
Copy link
Member Author

/retest

stevenctl
stevenctl reviewed Aug 23, 2021
View reviewed changes
Copy link
Contributor

@stevenctl stevenctl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what we actually want is to control this with kubectl.kubernetes.io/default-container

https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container

@silenceshell
Copy link
Member Author

I think what we actually want is to control this with kubectl.kubernetes.io/default-container

https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container

This is a good choice, my only concern is backward compatible, as default-container-annotation is introduced to k8s from v1.21, this may result in istio benchmark tools not working on older k8s clusters.

@astronaut0131
Copy link
Contributor 

-------------- Running in both mode --------------
e6f01502_qps_1000_c_2_1024_v2-stats-nullvmbothsidecars_perf.data
kubectl --namespace twopods-istio exec fortioclient-69bd59f79b-xkqn9  -- fortio load  -jitter=True -c 2 -qps 1000 -t 240s -a -r 0.001   -httpbufferkb=128 -labels e6f01502_qps_1000_c_2_1024_v2-stats-nullvm_both http://fortioserver:8080/echo?size=1024
OCI runtime exec failed: exec failed: unable to start container process: exec: "fortio": executable file not found in $PATH: unknown

Same problem here, runner.py can not even work correctly, why this PR is blocking?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenctl stevenctl stevenctl left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@silenceshell @istio-policy-bot @astronaut0131 @stevenctl @istio-testing